Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch to ParserExtensions #162

Closed
wants to merge 2 commits into from
Closed

Conversation

marklauter
Copy link
Contributor

Ideas for #87

I don't have tests for the changes yet.

@nblumhardt
Copy link
Member

Thanks for exploring this. I'm trying to get the context around this API paged back in 😅 - is there a use case that's currently valuable to you that this enables? Cheers!

@marklauter
Copy link
Contributor Author

I don't have a use case for it, but I've been interested in contributing to Superpower since you introduced me to the magic of parser combinators with one of your NDC talks on Youtube. So I'm looking at the open up-for-grabs items to see if I can help.

@nblumhardt
Copy link
Member

Ah, awesome! Thanks 😎

I think this is worth exploring further, but we'll need to find/wait for some motivating examples to make sure it hits the mark. Superpower's goal is to be more precise about reporting errors, so I wonder if the quick-and-dirty uses for Catch in Sprache might not appear so much with Superpower? 🤔

API wise, I think the examples would have to answer questions like:

  • Is it worth being generic over the exception type (which means specifying all type parameters explicitly when called)?
  • What's the equivalent of a "rethrow" in case the exception can't be handled?
  • Is Catch() any clearer or more precise than just Select(x => { try ...?

Probably a good time to park it and come back.

I haven't reviewed the "up-for-grabs" list for quite some time, I'll run through now and make notes on any that still seem good-to-go, in case there's something there you're interested to take a look at. Thanks again!

@marklauter
Copy link
Contributor Author

I didn't see Catch in Sprache (checked both develop and main branches).
The OP from Issue 87 mentioned SelectCatch which I did find in Serilog.Expressions.

I've been using Superpower to parse simple predicate expressions for a work project and to parse DDL and SQL for a Sqlite challenge on Code Crafters. So far I haven't encountered a use case for Catch, but all the operational code is under my control and I'm avoiding exceptions. Might be different if I had to use external code, or maybe network requests. The error messages generate by Superpower are already good and I think if you craft your expressions correctly, you should already handle the edge cases with OptionalOrDefault.

static class ParserExtensions
{
    public static TokenListParser<TTokenKind, TResult> SelectCatch<TTokenKind, TArg, TResult>(
        this TokenListParser<TTokenKind, TArg> parser, 
        Func<TArg, TResult> trySelector, 
        string errorMessage)
    {
        if (parser == null) throw new ArgumentNullException(nameof(parser));
        if (trySelector == null) throw new ArgumentNullException(nameof(trySelector));
        if (errorMessage == null) throw new ArgumentNullException(nameof(errorMessage));

        return input =>
        {
            var t = parser(input);
            if (!t.HasValue)
                return TokenListParserResult.CastEmpty<TTokenKind, TArg, TResult>(t);

            try
            {
                var u = trySelector(t.Value);
                return TokenListParserResult.Value(u, input, t.Remainder);
            }
            catch
            {
                return TokenListParserResult.Empty<TTokenKind, TResult>(input, errorMessage);
            }
        };
    }
}

@nblumhardt
Copy link
Member

HI @marklauter, it doesn't look like we'll push this PR forward for the time being; we're running through open PRs and cleaning up/triaging, I'll close this as not planned but we may revisit in the future. Thanks again!

@nblumhardt nblumhardt closed this Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants